[781] Add support for multiline labels #1087
Conversation
sbegaudeau
changed the title
lfasani
force-pushed
the
lfa/enh/borderNode2
branch
from
c4ebc5e to
d328757
Compare
lfasani
force-pushed
the
lfa/enh/multilineLabel
branch
2 times, most recently
from
ef37e1f to
55c90bb
Compare
| if (!node.isBorderNode()) { | ||
| // The label is positioned at the center of the node and the front-end will apply a "'text-anchor': | ||
| // 'middle'" property. | ||
| xOffSet = node.getLabel().getSize().getWidth() / 2; | ||
| } |
There was a problem hiding this comment.
Can make this hypothesis if in the class below, we have the code to deal with various label placement strategies? What was the issue with the previous algorithm?
There was a problem hiding this comment.
I did not catch your first sentence.
What was the issue with the previous algorithm?
The frontend needs to display the label area centered on the node and each line part centered inside the node
I used "text-anchor:middle" to do that. But to make it work the label position needs to be positionned on the center of the node. I tried to apply text-anchor:middle only on tspan instead of the parent text but text-anchor:middle make still the line positionned on the label x position
...c/test/java/org/eclipse/sirius/components/diagrams/layout/incremental/DiagramLayoutTest.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/eclipse/sirius/components/diagrams/layout/incremental/DiagramLayoutTest.java
Outdated
Show resolved
Hide resolved
...c/test/java/org/eclipse/sirius/components/diagrams/layout/incremental/DiagramLayoutTest.java
Outdated
Show resolved
Hide resolved
| public Diagram performElKLayout(IEditingContext editingContext, Diagram diagram) { | ||
| return this.layoutService.layout(editingContext, diagram); | ||
| } | ||
|
|
There was a problem hiding this comment.
used to perform the elk layout
...ents-diagrams/src/main/java/org/eclipse/sirius/components/diagrams/components/LabelType.java
Outdated
Show resolved
Hide resolved
| @@ -39,7 +39,7 @@ | |||
| public class EStringIfDescriptionProvider { | |||
| private static final String IF_DESCRIPTION_ID = "EString"; //$NON-NLS-1$ | |||
|
|
|||
| private static final String TEXTFIELD_DESCRIPTION_ID = "Textfield"; //$NON-NLS-1$ | |||
| private static final String TEXTAREA_DESCRIPTION_ID = "Textarea"; //$NON-NLS-1$ | |||
There was a problem hiding this comment.
Why does this file needs to be changed? Using a textfield does not change anything at all on the behavior of the frontend nor on this feature, right?
There was a problem hiding this comment.
Ths is needed because the frontend component TextfieldPropertySection checks the typeName if it equals 'Textarea'
sbegaudeau
force-pushed
the
lfa/enh/borderNode2
branch
from
d328757 to
134546e
Compare
lfasani
force-pushed
the
lfa/enh/multilineLabel
branch
from
55c90bb to
69bc5f4
Compare
lfasani
force-pushed
the
lfa/enh/multilineLabel
branch
2 times, most recently
from
1bd6265 to
ad373c4
Compare
The changes are about the node label center position. There is in fact no visible change but there are technical changes to prepare the following commits when we will have a label on many lines: * the label position given by the server is the horizontal center of the node instead of the left position of the label * consequently the LabelView is updated to display the text with 'text-anchor': 'middle' instead of the default 'text-anchor': 'start' The default form description will now use a text area for default form representation. It will allow multiple line edition. Shift + return in a text area only adds a line return but does not send the mutation. Adapt the ELK to consider the multiple line. The ELK label TextBound is set according the line returns contained in the label text. This will allow to have the right width of the node and to avoid the overlap of the label text with the contained nodes. Add a isMultiLine attribute to Label so that it can be seen as a text area by sprotty. Adapt the editing bounds and the edited text font to have always the same editing area whatever the zoom level. Bug: #781 Signed-off-by: Laurent Fasani <[email protected]>
sbegaudeau
force-pushed
the
lfa/enh/multilineLabel
branch
from
ad373c4 to
b4ac4ea
Compare
| const Text = () => { | ||
| const lines = text.split('\n'); | ||
| if (lines.length == 1) { | ||
| return text; | ||
| } else { | ||
| return lines.map((line, index) => { | ||
| if (index === 0) { | ||
| return <tspan x="0">{line}</tspan>; | ||
| } else { | ||
| if (line.length == 0) { | ||
| // avoid tspan to be ignored if there is only a line return | ||
| line = ' '; //$NON-NLS-1$ | ||
| } | ||
| return ( | ||
| <tspan x="0" dy={fontSize}> | ||
| {line} | ||
| </tspan> | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This component can be extracted and given props. Overwise its lifecycle is a bit odd since it is based on a closure
Type of this PR
Issue(s)
...
What does this PR do?
...
Screenshot/screencast of this PR
...
Potential side effects
...
How to test this PR?
Checklist
-s) with my mail address of my Eclipse Account.